Feat(Mapper): Support [Key] and [NotMapped] attributes from DataAnnotations #2722
Conversation
…ed]) as aliases for [BsonId] and [BsonIgnore] (for .NET 8+)
There was a problem hiding this comment.
Pull Request Overview
This PR introduces substantial enhancements to LiteDB, focusing on vector index support and improved attribute recognition for data mapping. The key changes include:
- Vector Index Support: Complete implementation of vector similarity search with HNSW-based indexing, supporting Cosine, Euclidean, and DotProduct metrics
- DataAnnotations Interoperability: Recognition of
[Key]and[NotMapped]attributes fromSystem.ComponentModel.DataAnnotations(NET8_0_OR_GREATER only) - Multi-Column Ordering: Enhanced query support for
ThenBy/ThenByDescendingoperations - Infrastructure Improvements: Conditional compilation for platform-specific optimizations, improved transaction handling, and test framework enhancements
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| LiteDB/Client/Mapper/BsonMapper.GetEntityMapper.cs | Adds conditional support for [Key] and [NotMapped] attributes as aliases for LiteDB attributes |
| LiteDB.Tests/Mapper/CustomMapping_Tests.cs | Adds test coverage for new DataAnnotations attribute support |
| LiteDB/Engine/Pages/BasePage.cs | Adds VectorIndex page type and registration |
| LiteDB/Document/BsonType.cs | Introduces Vector BsonType for native vector storage |
| LiteDB/Document/BsonVector.cs | New class representing vector values with proper equality/hashing |
| LiteDB/Client/Database/LiteQueryable.cs | Implements ThenBy/ThenByDescending and vector query methods |
| LiteDB/Engine/Disk/Serializer/BufferWriter.cs | Splits string writing logic into platform-specific partial classes |
| LiteDB.Tests/Query/VectorIndex_Tests.cs | Comprehensive test suite for vector index functionality |
| LiteDB/Engine/Engine/*.cs | Integration of VectorIndexService across insert/update/delete operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| { | ||
| #if NET8_0_OR_GREATER | ||
| var idAttrs = new List<Type> { typeof(BsonIdAttribute), typeof(KeyAttribute) }; | ||
| var ignoreAttrs = new List<Type> { typeof(BsonIgnoreAttribute), typeof(NotMappedAttribute) }; | ||
| #else | ||
| // in netstandard2.0 KeyAttribute and NotMappedAttribute are not available without adding extra dependencies | ||
| var idAttrs = new List<Type> { typeof(BsonIdAttribute) }; | ||
| var ignoreAttrs = new List<Type> { typeof(BsonIgnoreAttribute) }; | ||
| #endif |
There was a problem hiding this comment.
Make them
- Private static fields
- Netstandard: Use reflection to find them maybe?
- (Optional)Configurable from outside - maybe users want to define them (just an idea)
There was a problem hiding this comment.
Hi @JKamsker in the original method there are a few local variabiables for managing those and others attributes. Do you want to make all of them static?
Using reflection without type checking means rely on string representation of such attributes. Even if it is possible it doesn't sound as a good design choice.
The configuration of custom attributes is possible and should be possible to ne managed in the BsonMapper.Global.
I'd rather open a separate issue for developments.
There was a problem hiding this comment.
Hi @JKamsker in the original method there are a few local variabiables for managing those and others attributes. Do you want to make all of them static?
If they dont change and are readonly? Absolutely yes!
Using reflection without type checking means rely on string representation of such attributes. Even if it is possible it doesn't sound as a good design choice.
<NET 8 is not a priority in terms of performance. We want feature parity between those builds, even if it costs us performance in old versions. Try to use reflection sparingly tho - cache what you can. And only for sub net8.
There was a problem hiding this comment.
Way too much change here - what's going on?
There was a problem hiding this comment.
thought to add a v6 section with details of the improvements, but some layout fixes from the md editor also came in. I revert to previous version with only the v6
|
Thanks for your contribution! Please see my comments. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
This PR enhances the
BsonMapperto recognize common attributes fromSystem.ComponentModel.DataAnnotations, improving interoperability with other frameworks like Entity Framework.Changes:
[Key]as[BsonId]: The[Key]attribute is now recognized as an alias for[BsonId(AutoId = false)].[NotMapped]as[BsonIgnore]: The[NotMapped]attribute is now recognized as an alias for[BsonIgnore].To avoid adding new dependencies to older target frameworks (like .NET Standard 2.0), this functionality is conditionally compiled and only active for projects targeting .NET 8 or newer (
#if NET8_0_OR_GREATER).Verification:
Added new unit tests in
CustomMapping_Tests.csto verify that[Key]and[NotMapped]attributes are correctly handled.Added a new benchmark (
InsertionIgnoreExpressionPropertyComponentModelBenchmark) to measure insertion performance when using property exclusion attributes.